[FLINK-37925][table] Reject out-of-range VARIANT casts and make VARIANT-to-string a value cast#28758
[FLINK-37925][table] Reject out-of-range VARIANT casts and make VARIANT-to-string a value cast#28758raminqaf wants to merge 7 commits into
Conversation
beec0e0 to
e03672b
Compare
f02dbdc to
49d4bca
Compare
3750b25 to
f9bd3a0
Compare
| ```sql | ||
| CAST(CAST(PARSE_JSON('1000') AS INT) AS TINYINT) -- returns -24 (wrap-around) | ||
| ``` | ||
| {{< /hint >}} |
There was a problem hiding this comment.
what is difference here from non VARIANT case?
02f5ffc to
e9c07c4
Compare
| LogicalType inputLogicalType, | ||
| LogicalType targetLogicalType) { | ||
| return methodCall(inputTerm, "toString"); | ||
| return staticCall(VariantCastUtils.class, "toStringValue", inputTerm); |
There was a problem hiding this comment.
according to LogicalTypeCasts, we support VARCHAR and CHAR both with various length. this cast rule should consider the target length as well. I guess it should throw if length exceeds?
There was a problem hiding this comment.
when the target is bounded CHAR(n)/VARCHAR(n), CharVarCharTrimPadCastRule kicks in, calls the variant→string rule, and trims/pads — the identical path every other to-string cast uses.
I added tests to prove it, and they pass:
- CAST(PARSE_JSON('"foobar"') AS VARCHAR(3)) → foo (trimmed)
- CAST(PARSE_JSON('"ab"') AS CHAR(5)) → ab (padded)
There was a problem hiding this comment.
Reverted the change. Now we follow this rule:
- VARCHAR(n): OK if length ≤ n, else throw.
- CHAR(n): OK only if length == n, else throw.
So CAST (PARSE_JSON('ab') AS VARCHAR(3)) -> returns ab
0a0d8d0 to
8f09c44
Compare
4dc9135 to
23e41c6
Compare
twalthr
left a comment
There was a problem hiding this comment.
Another round of comments.
| return staticCall(VariantCastUtils.class, "toLongExact", number); | ||
| case FLOAT: | ||
| return "floatValue"; | ||
| return methodCall(number, "floatValue"); |
There was a problem hiding this comment.
Are there any data loss checks in this method? Asking AI leads to exactness loss.
can I store a full long in a float?
The short answer is yes, in terms of size (range), but no, in terms of exactness (precision).
If you attempt to store a large long inside a float, the application will not crash, but you will likely lose data through rounding errors.
| return methodCall(number, "floatValue"); | ||
| case DOUBLE: | ||
| return "doubleValue"; | ||
| return methodCall(number, "doubleValue"); |
There was a problem hiding this comment.
Are there any data loss checks in this method? Asking AI leads to exactness loss.
|
|
||
| public static DecimalData toDecimalExact(Number value, int precision, int scale) { | ||
| final DecimalData decimal = | ||
| DecimalData.fromBigDecimal(toBigDecimal(value), precision, scale); |
There was a problem hiding this comment.
Where is max precision of 38 checked?
There was a problem hiding this comment.
Max precision 38 is guaranteed by the target DecimalType itself (DecimalType.MAX_PRECISION), so precision here is always ≤ 38. DecimalData.fromBigDecimal returns null when the value doesn't fit the given precision/scale, and we throw on that. So there's no separate check to add.
… fit the target type and allow string cast Follow-up to the initial VARIANT-to-primitive cast support. Numeric casts from a VARIANT now reject values that do not fit the target type instead of silently wrapping: an out-of-range integer or an overflowing DECIMAL fails CAST and returns NULL for TRY_CAST, matching Spark's variant cast behavior. FLOAT and DOUBLE keep lenient IEEE conversion, where overflow becomes infinity. The new VariantCastUtils performs the checked narrowing by truncating toward zero and range-checking the value. CAST(VARIANT AS CHAR/VARCHAR) is now allowed and returns the JSON string representation, so the previous JSON_STRING-only restriction and its cast hint are removed. VARIANT cast validation is expressed directly in the per-target rules of LogicalTypeCasts. The VARIANT section of the data types reference documents the overflow behavior and the double-cast pattern for wrap-around narrowing.
…pe mapping Describe which value kinds a VARIANT can hold, including that TIMESTAMP and TIMESTAMP_LTZ use microsecond precision and DATE a day count, and that there is no TIME kind. Add a table showing how PARSE_JSON maps JSON values to variant kinds, and explain that PARSE_JSON cannot produce FLOAT, DATE, TIMESTAMP, TIMESTAMP_LTZ, or BYTES because JSON has no literal for them; those kinds come from other producers such as VariantBuilder or format conversions. Point the PARSE_JSON function reference to the VARIANT data type section for the mapping details.
…nt interface Note that getDateTime and getInstant return values with microsecond precision, which the LocalDateTime and Instant return types do not convey on their own. Fix the getInstant javadoc to reference Type.TIMESTAMP_LTZ, the type it actually accepts, instead of Type.TIMESTAMP.
23e41c6 to
ac56197
Compare
What is the purpose of the change
Follow-up to the initial
VARIANT-to-primitive cast support (FLINK-37925). It tightens the cast semantics, turns the string cast into a real value cast, adds a decodability check, and documents the resulting behavior.VARIANTnow reject values that do not fit the target instead of silently wrapping. An out-of-range integer or an overflowingDECIMALfailsCASTand returnsNULLforTRY_CAST.FLOATandDOUBLEkeep lenient IEEE conversion, where overflow becomes infinity. This follows the behavior of Spark's variant casts.CAST(VARIANT AS CHAR/VARCHAR)now extracts the scalar value, so a stored string is returned unquoted (foo), while objects and arrays return their JSON representation.JSON_STRINGremains the way to get the JSON text, where a string stays quoted ("foo"). Previously the cast reused the JSON serialization and was indistinguishable fromJSON_STRING.Brief change log
VariantCastUtils(flink-table-runtime): range-checked numeric narrowing (truncate toward zero, throw on overflow) and scalar-to-string extraction.VariantToPrimitiveCastRule: route integer andDECIMALtargets through the checked helpers; keepFLOAT/DOUBLElenient.VariantToStringCastRule: value extraction instead of JSON serialization;JSON_STRING(JsonStringCallGen) is left unchanged.LogicalTypeCasts: expressVARIANTcast validation in the per-target rules and drop theJSON_STRINGcast hint.Variant.getInstantjavadoc to referenceType.TIMESTAMP_LTZand document microsecond timestamp precision.VARIANTvalue encoding, thePARSE_JSONtype mapping, the string-cast vsJSON_STRINGdistinction, and thatNaN/infinity are not valid JSON (witha store-as-string workaround).Verifying this change
This change added and updated tests:
CastFunctionITCase: overflow cases (CASTfails,TRY_CASTreturnsNULL), lenientFLOAT/DOUBLE, and string-cast value extraction (a string returns unquoted, objects return JSON).LogicalTypeCastsTest: theVARIANTcast-support matrix.BinaryVariantInternalBuilderTest:PARSE_JSONrejectsNaN,Infinity, and-Infinity.BinaryVariantTest:checkFullySupportedaccepts supported types and rejects every primitive type-code above 16.Does this pull request potentially affect one of the following parts:
@Public(Evolving): yes, javadoc-only change to the@PublicEvolvingVariantinterface (no signature change)VARIANT-to-numeric cast pathDocumentation
Was generative AI tooling used to co-author this PR?
Generated-by: Opus 4.8